Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add Weightless benchmark bailing #12829

Merged
merged 4 commits into from
Dec 2, 2022
Merged

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Dec 2, 2022

The (child)-bounties benchmark require SpendOrigin as dispatch origin. Since SpendOrigin is configured to NeverEnsureOrigin in Polkadot, it is impossible to make these Calls since no origin instance can be created.

This MR fixes this by:

  • Adding a Weightless early return option for benchmarks whose weight will be annotated with 0. It is not completly zero since users still pay the ExtrinsicBaseWeight and a length-based fee.
  • Using this option in the bounties and child-bounties benches.

This is a better solution than #11562 since there would be a lot of ifs needed otherwise.

TODO:

These are not actually companions but needed to make the CI green:
Polkadot companion: paritytech/polkadot#6328
Cumulus companion: paritytech/cumulus#1940

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Dec 2, 2022
frame/benchmarking/src/lib.rs Outdated Show resolved Hide resolved
for i in 0..n {
let (caller, _curator, _fee, value, reason) =
setup_bounty::<T, I>(i, T::MaximumReasonLength::get());
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
let bounty_id = BountyCount::<T, I>::get() - 1;
let approve_origin = T::ApproveOrigin::successful_origin();
let approve_origin =
T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably change all benchmarking code to use try_successful_origin and remove successful_origin?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes eventually… this was an oversight when originally writing them.

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez ggwpez added B7-runtimenoteworthy C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Dec 2, 2022
@ghost ghost self-requested a review December 2, 2022 19:44
@ggwpez
Copy link
Member Author

ggwpez commented Dec 2, 2022

bot merge

@paritytech-processbot
Copy link

Error: "Check reviews" status is not passing for paritytech/polkadot#6328

@ggwpez ggwpez merged commit 34900ca into master Dec 2, 2022
@ggwpez ggwpez deleted the oty-short-circuit-bounties-bench branch December 2, 2022 20:14
@ggwpez
Copy link
Member Author

ggwpez commented Dec 8, 2022

@coderobe this should set the weights for many of the (child)-bounty calls to zero for the Polkadot and Rococo runtime.
Just so that you are aware when you see it in the next update 😄

ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
* Calls can be 'Weightless'

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Fix (child)-bounties benches

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Just use one dummy value

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* 🤦

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* Calls can be 'Weightless'

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Fix (child)-bounties benches

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* Just use one dummy value

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

* 🤦

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez ggwpez mentioned this pull request Mar 7, 2023
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants